Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great step towards improving test coverage by adding a comprehensive suite of integration tests. However, I've identified a critical security vulnerability due to hardcoded credentials in a test helper file, which must be addressed immediately. There is also a critical bug in the createTestFolder helper function that will cause several tests to fail. Additionally, I found several high-severity inconsistencies where the integration tests are using different API endpoints than the corresponding tool wrappers, which undermines the goal of verifying the tools' correctness. I've provided detailed comments and suggestions to resolve these issues.
| expect(response.data.projects.length).toBeGreaterThan(0) | ||
|
|
||
| // Verify the test project is in the list | ||
| const testProject = response.data.projects.find((p: any) => p.code === testProjectCode) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Add typecheck step to .husky/pre-commit hook - Fix TypeScript type error in integration test helpers - Consolidate integration tests into main CI job (previously separate) - Add integration test documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Separate vitest configs for unit and integration tests - Add guards in afterAll hooks to prevent cleanup errors when beforeAll fails - Use getter functions for env vars to support dotenv loading - Remove hardcoded credentials (API_KEY, AUTH_EMAIL, AUTH_PASSWORD now required) - Add .env.example for local development - Update CI workflow to include QASPHERE_AUTH_EMAIL secret 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
- Add retry logic with exponential backoff for login (up to 5 retries) - Disable file parallelism to prevent concurrent login attempts - Detect rate limits via HTTP 429 or "too many requests" message 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
- Fix API endpoints to match actual tool implementations: - helpers.ts: /folder/bulk-upsert -> /tcase/folder/bulk - customFields: /tcase/custom-fields -> /custom-field - tags: /tcase/tags -> /tag - requirements: /tcase/requirements -> /requirement - Fix response format expectations: - customFields: response.data.customFields (not array), field.name (not title) - tags: response.data.tags (not array) - requirements: response.data.requirements (not array) - tcases: response.data.id (not tcaseId), status 201 for POST - Fix array params serialization using URLSearchParams for correct repeated param format (include=steps&include=tags) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
satvik007
requested review from
AndrianBdn,
hi-rai,
ramilamparo and
sirkostya009
| expect(response.status).toBe(200) | ||
| expect(response.data).toHaveProperty('projects') | ||
| expect(Array.isArray(response.data.projects)).toBe(true) | ||
| expect(response.data.projects.length).toBeGreaterThan(0) | ||
|
|
||
| // Verify the test project is in the list | ||
| const testProject = response.data.projects.find((p: any) => p.code === testProjectCode) | ||
| expect(testProject).toBeDefined() | ||
| expect(testProject.id).toBe(testProjectId) |
There was a problem hiding this comment.
I don't understand what's the point of testing API calls with axios? These tests seems out of scope for this project.
Shouldn't we test tool calls instead to make sure the tools are calling the APIs correctly?
| QASPHERE_AUTH_EMAIL: ${{ secrets.QASPHERE_AUTH_EMAIL }} | ||
| QASPHERE_AUTH_PASSWORD: ${{ secrets.QASPHERE_AUTH_PASSWORD }} |
There was a problem hiding this comment.
Why do we need email and password? Using an API key seems enough. We never use username and password for public APIs.
2 participants
Summary
Adds integration tests for all public API endpoints using Vitest.
Changes
Run locally
Requires env vars:
QASPHERE_API_KEY,QASPHERE_AUTH_EMAIL,QASPHERE_AUTH_PASSWORD